-
Notifications
You must be signed in to change notification settings - Fork 5.9k
GH-673: Revert "Remove chmod on project dir" #679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Due to the decision to remove the mode change, this inheriintly reintroduced a bug that prevents the system to write to the bind mmounted folder. the chmod is intended to *fix* it. This reverts commit cdc5b55.
How does this fix #676? In fact, if you don't have read perms for the folder, how would the chmod even succeed? |
@nhooyr if I got the writing part for this right, you still have ownership in the folder because you're inside |
@nhooyr, for what it is worth I would like to see this approved or denied as I am experiencing the same issue: |
Then why do you need to make it group editable if you have ownership? |
Apparently this does not mandate the ownership well for bind mounts for some reason so explicitly setting them as RW is the only way to make sure you can write changes from container to host @nhooyr. |
We need to know what the reason is before merging this. The only reason I see is if the directory outside of the container has a different owner than uid 1000 at which point you should be changing the directory outside of the container or run code-server as a different user instead of uid 1000. See #640 |
it will "Permission denied" on docker |
1 similar comment
it will "Permission denied" on docker |
Even if we set it to UID1000, the permissions in the bind mounted folder is not the same, therefore, we should force writes and reads on the bind folder so we can write to the folder. |
There is a inherent reason we do this as well, permissions is irrelevant to the host because its a container-specific permission. There will be no impact on security since it only applies to the container. |
I don't think this is true, if its mounted in, then any changes to it will affect the folder outside the container. Docker doesn't have user namespacing enabled by default, so the UIDs/GIDs inside the container are the same as the UIDs/GIDs outside. Can you show the perms on the folder you're having trouble with and then show the UID the container is running as? Do a |
In fact can you open a new issue? A issue is better suited for debugging than this PR. |
Revisiting this PR due to GH-992. Impact is considered major. |
@marcdumais-work Provides a explanation over at our friends at Theia, which you can find here.
So essentially speaking in the terms of Unix permission model we do not have the ability to write to that folder even if we own |
This wouldn't work regardless. If we do something like this, it has to be done in the |
Describe in detail the problem you had and how this PR fixes it
Due to the decision to remove the mode change, this inherently
reintroduced a bug that prevents the system to write to the bind
mounted folder. the chmod is intended to fix it.
Is there an open issue you can link to?
this fixes GH-673